tests/src/ui/UICanvas.test.ts (1)

354-389: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a second element to prevent hiding concurrent modification bugs.

Testing the demote cascade with only a single nested element (image) masks concurrent modification bugs (such as removing elements from an array while looping over it, which succeeds for length 1 but skips elements for length 2+). Adding a second element ensures the logic robustly processes all children and would successfully catch the forEach iteration bug in UICanvas.ts.

♻️ Proposed test update

   it("re-homes elements to the new root canvas when theirs loses root status", () => {
     const outerEntity = root.createChild("outer");
     const innerEntity = outerEntity.createChild("inner");
     const innerCanvas = innerEntity.addComponent(UICanvas);
     const imageEntity = innerEntity.createChild("image");
     const image = imageEntity.addComponent(Image);
+    const image2Entity = innerEntity.createChild("image2");
+    const image2 = image2Entity.addComponent(Image);
 
     // Collected by the inner canvas while it is the root.
     // `@ts-ignore`
     innerCanvas._getRenderers();
     // `@ts-ignore`
     expect(image._getRootCanvas()).to.eq(innerCanvas);
+    // `@ts-ignore`
+    expect(image2._getRootCanvas()).to.eq(innerCanvas);
     // `@ts-ignore`
     expect(innerCanvas._disorderedElements.length).to.be.greaterThan(0);
 
     // Enabling a canvas on an ancestor demotes the inner one.
     const outerCanvas = outerEntity.addComponent(UICanvas);
     // `@ts-ignore`
     expect(innerCanvas._isRootCanvas).to.be.false;
     // `@ts-ignore`
     expect(outerCanvas._isRootCanvas).to.be.true;
     // The element's canvas state must be reset (the walk's re-assignment is gated on the
     // dirty flag), otherwise it keeps rendering into the demoted canvas's dead queue.
     // `@ts-ignore`
     expect(image._isRootCanvasDirty).to.be.true;
     // `@ts-ignore`
     expect(image._rootCanvas).to.be.null;
+    // `@ts-ignore`
+    expect(image2._isRootCanvasDirty).to.be.true;
+    // `@ts-ignore`
+    expect(image2._rootCanvas).to.be.null;
 
     // The new root adopts it on its next walk.
     // `@ts-ignore`
     outerCanvas._getRenderers();
     // `@ts-ignore`
     expect(image._getRootCanvas()).to.eq(outerCanvas);
+    // `@ts-ignore`
+    expect(image2._getRootCanvas()).to.eq(outerCanvas);
 
     outerEntity.destroy();
   });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/src/ui/UICanvas.test.ts` around lines 354 - 389, Add a second child UI
element with an Image component in the test “re-homes elements to the new root
canvas when theirs loses root status,” then assert both images are marked dirty,
have null root canvases after demotion, and are reassigned to outerCanvas after
its renderer walk. Keep the existing assertions and cleanup behavior intact.